Conversation
c-cube
commented
Apr 21, 2026
c-cube
commented
- modifies how loggers work so it's easier to swap them at runtime
- pass timestamp and a list of key/value pairs
- normal callsites for logging are unaffected, but it enables us to have richer logging.
c-cube
force-pushed
the
sc/structured-logging
branch
from
c2eced1 to
07ed94a
Compare
c-cube
force-pushed
the
sc/structured-logging
branch
from
de9c75f to
ba7e6f4
Compare
the goal is to preserve the current behavior, but enable: - alternative formats (logfmt, later ndjson/jsonl) - runtime swap of log format, no functor
easier on angle-grinder and on the eyes in general
c-cube
force-pushed
the
sc/structured-logging
branch
from
ba7e6f4 to
c87470a
Compare
|
GHA/ocaml-setup is doing weird stuff, but otherwise it's ready for review. |
|
will merge wed 5/6 |
rr0gi
left a comment
rr0gi
left a comment
There was a problem hiding this comment.
i don't like that it changes the format of default logging
i would suggest having another parameter that keeps old message in default logfmt and uses pairs in rich (in addition to current ~pairs which i understand appends pairs to log message always)
| let do_fork () = | ||
| match check_req req with | ||
| | `Error err -> Exn.fail "pre fork %s : %s" (show_request req) (Unix.error_message err) | ||
| | `Error err -> Exn.fail "pre fork : %s" (show_request req) (Unix.error_message err) |
| let verbose_curl_result nr_http action t h code = | ||
| let open Curl in | ||
| let b = Buffer.create 10 in | ||
| bprintf b "%s #%d %s ⇓%s ⇑%s %s " |
There was a problem hiding this comment.
so how will Web request log look like now?
|
Ok, I'll keep the old code and switch between the two based on the current log format, to get 100% compatibility with existing logs.
|
|
Ok, the web and http logs should be back to (almost) exactly what they were, modulo a bit of punctuation (some ":" added). The logging is more uniform now, and checks what logging backend we use to log accordingly. |
c-cube
force-pushed
the
sc/structured-logging
branch
from
06db0f7 to
0a08364
Compare
| Hashtbl.remove c.server.reqs req.id; | ||
| if c.server.config.debug then | ||
| log #info "finished %s" (show_request req) | ||
| log_req `Info req "finished" |
There was a problem hiding this comment.
i lowkey hoped it is possible to preserve log #info way
There was a problem hiding this comment.
the problem is that it's better to push info into key/value pairs, so they're easily queryable, so we can't have a single format string for both cases
2 participants
